Skip to content

Upgraded missing numba to ModuleNotFoundError instead of a warning.#4291

Closed
pauladkisson wants to merge 1 commit intoSpikeInterface:mainfrom
catalystneuro:fix_rp_violations
Closed

Upgraded missing numba to ModuleNotFoundError instead of a warning.#4291
pauladkisson wants to merge 1 commit intoSpikeInterface:mainfrom
catalystneuro:fix_rp_violations

Conversation

@pauladkisson
Copy link
Collaborator

@pauladkisson pauladkisson commented Jan 5, 2026 •
edited
Loading

Fixes #4290

Note that metric.compute is actually handled inside a try-except block. So this still only raises a warning when Numba is not installed. Just a more informative one.

spikeinterface/src/spikeinterface/core/analyzer_extension_core.py

Lines 1127 to 1141 in 53c247a

try:
metric_params = self.params["metric_params"].get(metric_name, {})
res = metric.compute(
sorting_analyzer,
unit_ids=unit_ids,
metric_params=metric_params,
tmp_data=tmp_data,
job_kwargs=job_kwargs,
)
except Exception as e:
warnings.warn(f"Error computing metric {metric_name}: {e}")
if len(column_names) == 1:
res = {unit_id: np.nan for unit_id in unit_ids}
else:
res = namedtuple("MetricResult", column_names)(*([np.nan] * len(column_names)))

@alejoe91 alejoe91 added the metrics Related to metrics module label Jan 6, 2026
@alejoe91
Copy link
Member

alejoe91 commented Jan 6, 2026

@chrishalcrow since you said you were on it, wanna have a look? @pauladkisson beat you at the PR too ehehehe

@alejoe91 alejoe91 mentioned this pull request Jan 6, 2026
Merged
@chrishalcrow
Copy link
Member

chrishalcrow commented Jan 6, 2026

Hey @pauladkisson - thanks for the PR! We don't generally raise errors if there's an issue with one quality metric. This is because we don't want an entire pipeline to fail if one metric fails. Hence I suggest #4292 , which will return nans if numba is not installed.

Another option: @alejoe91 can we make "numba" depend_on? Then the metric finder would ignore the metric.

@samuelgarcia
Copy link
Member

samuelgarcia commented Jan 9, 2026

Another option: @alejoe91 can we make "numba" depend_on? Then the metric finder would ignore the metric.

I think would prefer an explicit _module_dependencies attribute in the class. This can be None by default and then the change is minnial and so we can skip/ignore it without raising errors.

@pauladkisson
Copy link
Collaborator Author

pauladkisson commented Jan 15, 2026

Superseded by #4292

@pauladkisson pauladkisson deleted the fix_rp_violations branch January 15, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alejoe91 alejoe91 alejoe91 approved these changes

Assignees

No one assigned

Labels

metrics Related to metrics module

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

RPViolation quality metric throws an opaque error if Numba is not installed.

4 participants

@pauladkisson @alejoe91 @chrishalcrow @samuelgarcia